Symbol#source correct for all phases #6757
Conversation
| def check(src: SourceFile): Boolean = | ||
| src.exists && isSimilar(dPath, src.path) | ||
|
|
||
| assert(d.symbol.source.exists && (check(ctx.owner.source) || check(ctx.compilationUnit.source)), | ||
| s"private ${d.symbol.showLocated} in ${d.symbol.source} accessed from ${ctx.owner.showLocated} in ${ctx.owner.source}") |
There was a problem hiding this comment.
Should this not be always ctx.compilationUnit.source? For the purposes of the bytecode the current owner does does not matter, I think. Have you tried to simply change ctx.owner.source to ctx.compilationUnit.source?
There was a problem hiding this comment.
I did try that initially. It doesn't work because if there's inlining on the d side then post-erasure the source of the symbol might be different from the compilation unit.
There was a problem hiding this comment.
An alternative might be to check d.defTree.source against ctx.compilationUnit.source ... does that sound reasonable?
There was a problem hiding this comment.
defTree does not always exist. So, I don't see a way to simplify the current state either.
|
@odersky I wasn't really happy with the PR as it stood, so I did some more digging. It turns out that the inliner was missing the opportunity to assign Also, we can't always use I've tweaked the inliner to set defTrees where possible, and I've also pulled the source file computation out ( |
|
I've moved the @odersky this feels like a better and more principled fix ... WDYT? |
milessabin
changed the title
|
@nicolasstucki thanks ... I'll squash and merged :-) |
A combination of the inliner missing opportunities for setting defTrees with the correct inlined source information and some suspect post erasure logic in Symbol#source led to the same-source check in ExpandPrivate failing in joint-compilation scenarios resulting in multi-file programs being incorrectly rejected for access violations.
milessabin
force-pushed
the
topic/inline-refinement-by-name
branch
from
3718405 to
cf44e91
Compare
|
Squashed and rebased ... I'll merge when it goes green. |
A combination of the inliner missing opportunities for setting
defTreeswith the correct inlined source information and some suspect post erasure logic inSymbol#sourceled to the same-source check inExpandPrivatefailing in joint-compilation scenarios resulting in multi-file programs being incorrectly rejected for access violations.The inliner duplicates trees and their symbols when expanding inline definitions at call sites. When the RHS of the inline definition contains a class definition itsSymbolis duplicated, I believe correctly, preserving the original position and associated source file. The tree is duplicated, I believe also correctly, with its source attribute reflecting that the tree has been expanded in the call site source file.In most circumstances the potential mismatch between the source of theSymboland the source of its defining tree is handled: inSymbol#sourcethe source of the defining tree will be used prior to erasure if the tree is non-empty, which ensures that the source as viewed via theSymbolwill be the same as the source as viewed via the tree.Things can go awry after erasure, however. InExpandPrivatethere is a same-source-file test relaxing access controls to private members of module classes. Because this happens after erasure it's possible that if a class definition has been inlined into a module and members attempt to access a definition in the module (eg. something synthetic which has been lifted into the module) the same-source-file test will fail because the source of the class symbol will now be seen as the source of the inline definition rather than the source of the inlined call site.This is the scenario exercised in the included test. The inlined refinement and by-name argument are both essential (the latter is responsible for the private synthetic term which is lifted out into the module and then incorrectly reported as inaccessible from the inlined body). Also note that multiple source files are essential, but that separate compilation (as opposed to joint compilation) masks the issue which is why this problem didn't show up for the multi-file version of my type class derivation prototype.In the real code from which this was reduced the refinement was actually the expansion of a polymorphic function type, but as the test shows there's nothing specific to polymorphic function types here.The simplest approach to fixing this appears to be inExpandPrivatewhere if the accessing symbol (on thecontext.ownerside) doesn't have a matching source file we fall back to checking against the source file of the current compilation unit.